Handle Empty Messages #82
Conversation
|
My tests all pass but I want to sub this in a couple places and actually 'see' it then I'll re-open. But it's ready for review and I can just delete any parts you don't like. :-) 👍 |
|
i have confirmed, at least in 2 small utilities, that this behaves as expected and as the tests describe. |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #82 +/- ##
==========================================
+ Coverage 77.63% 77.95% +0.31%
==========================================
Files 10 10
Lines 635 644 +9
==========================================
+ Hits 493 502 +9
Misses 128 128
Partials 14 14
☔ View full report in Codecov by Sentry. |
|
reviewed, i think this is ok. coverage is still 'up' and the lines that are 'missed' are the same as the rest of the cases, just for timestamp. i expect getting timestamp without level or message or kv is unlikely to occur that often and would take a little setup to even test. |
| if !firstKey { | ||
| l.b.WriteByte(' ') | ||
| } |
There was a problem hiding this comment.
Perhaps put this in a helper function writeSpace(bool)
|
Thanks for the PR @dezren39. Some comments, looks good otherwise :) |
resolves #78
This PR handles empty messages, so that you can use 'just' structured logging while using the text formatter, by skipping the message block.
This pr also aligns all the cases to 'add a space when you need one for this loop' instead of some being that way and others being 'add a space because there will be a next loop'.
An impact to non-text logger, if you provide empty message through message parameter no message is sent. I think if you sent a 'msg' key with an empty string that would still end up in the file.
I gitignored .vscode .history and go.work, sometimes they are needed but they get accidentally committed randomly so it made sense to me to ignore them until there is an active file to be committed for those.
I added 3 new tests, and didn't make any new tests fail,,, except that I have some tests that don't work for me already. #81 was opened for that. Maybe that's my issue? I didn't look into it too much when main branch also didn't succeed.